Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Video trimming UI plugin #1798

Open
wants to merge 24 commits into
base: develop
Choose a base branch
from

Conversation

DanielHuntleySBG
Copy link
Contributor

Link to related issue (if applicable)

1743

Summary of proposed changes

Introduction of a video trimming tool into the libary. The tool is useful when creating clips or repeating loops.

Workflow

  • The trimming tool can be toggled via the enterTrim/exitTrim event or using the control in the controls bar.
  • Users can then drag the start and end point of the region to define the clipping region.
  • Doing so fires the trimChange event outputting the start and end points of the region.
  • From here the defined bounds of the clipping region can be used to create a clip (which will need to be done by the user)

Covered edge cases:

  • The user cannot define the trimming region outside of the bounds of the timeline.
  • The start or end time cannot be before one another

trimmingExample

Thanks for taking the time to review these proposed changes and welcome any feedback or changes, thanks!

Checklist

  • Use develop as the base branch
  • Exclude the gulp build (/dist changes) from the PR
  • Test on supported browsers

Danielh112 and others added 23 commits April 2, 2020 14:28
… outside of the trimming region and set to the start of the trimming region
…g the trimming length (let go of trimming handle)
…y the seek position in the video | Trim plugin is also brought to be more inline with the existing code base (sass structure and change of this.tool to be this.elements)
…was not created | We should only display the trim thumb time tooltip when displaying preview thumbnails as a seek tooltip is displayed
…in the correct format and pass a valid element
…tting the trim length | Prevent the end value of the trimming region from being before the start
…sed via the trimming object and update documentation
…om the trim class: unsure whether a setter should exist for both of these and what its naming convention would be, so have kept it set via the variables directly
… so have moved the logic to inside the enter and exit function | Additionally removed toggling trimming from plyr.js as it can all be set via trim.xxx
@tomByrer
Copy link

@DanielHuntleySBG
Copy link
Contributor Author

Interesting idea! Does it use Chapters?

http://thenewcode.com/977/Create-Interactive-HTML5-Video-with-WebVTT-Chapters

http://html5videoguide.net/demos/google_io/3_navigation/#videoBox

It doesn't at the moment but that would be a good accessbilility enhancement!

@tomByrer
Copy link

tomByrer commented May 3, 2020

"Chapters" is for more than accessibility; some tools use that markup..

@Thiryn
Copy link

Thiryn commented Jul 21, 2020

Chapters are a good idea but perhaps not part of his work, this is allowing to make a 'sub-timeline', I like the idea and looks good. @sampotts any plan to bring this in?

@jackguoAtJogg
Copy link

@DanielHuntleySBG Do you have documentation how to use it? I would like to try it out since I have a project that needs a trimmer like this.

@jackguoAtJogg
Copy link

@sampotts Do we a timeline that when will this get merged?

@DanielHuntleySBG
Copy link
Contributor Author

@jackguoAtJogg I have added documentation in the readme file outlining listeners and methods, hope it helps

@jackguoAtJogg
Copy link

@DanielHuntleySBG I see it in the README, I'll play around with it! Thank you making this amazing UI plugin! 🙇

@adeeb1
Copy link

adeeb1 commented Sep 26, 2020

@DanielHuntleySBG I would love to use this. Would it be possible to resolve conflicts and update the branch please?

@DanielHuntleySBG
Copy link
Contributor Author

@adeeb1 Hello, I would be happy to resolve the merge conflicts but am still waiting to hear whether it will be merged into the main repo 😄

@adeeb1
Copy link

adeeb1 commented Oct 5, 2020

@DanielHuntleySBG If it's not too much work, this would be valuable to me (and possibly others) even if it's not merged into the official repo. I'm already using a fork with Dailymotion support (credit) and could include this in it. Otherwise, I may be able to update this.

@sampotts
Copy link
Owner

Could you fix the conflict on this? Then we could look at merging it in.

@DanielHuntleySBG
Copy link
Contributor Author

@sampotts Should be resolved, let me know if you have any issues

@sampotts
Copy link
Owner

Thanks! Will take a look.

@jackguoAtJogg
Copy link

jackguoAtJogg commented May 12, 2021

@DanielHuntleySBG I believe there's a typescript bug in plyr.d.ts line 834 where the duration type is missing.

interface SyncPoint {
    id: string;
    time: number;
    start: number;
    duration: duration;
  }

@dy
Copy link

dy commented Jun 17, 2021

Any chance it will be merged? Missing this feature in coming project, youtube clips is also in early alpha. What help can I provide?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants